Split out MerkleTree contract and emit CapeBlock as part of BlockCommitted event #1097

sveitser · 2022-05-27T14:27:46Z

I was curious to how much work it would be to make this change but it seemed quicker to just try to do it. We still need to discuss if it's a good idea to make this change, especially at this late stage.

If we decide to do this the calldata vulnerability does no longer exist. We should still make sure the EQS doesn't panic if something is wrong with the memos though. Those are still stored in calldata.

Close #1085

TODO

Update hardcoded address of CAPE contract. This changes because we have to deploy one more contract. Unless we deploy it with a different account.
Update .env.development in UI repo with new contract address 0x20Dc424c5fa468CbB1c702308F0cC9c14DA2825C: https://github.com/EspressoSystems/cape-ui/pull/438
Add a test to check if access control of merkle tree contract prevents other addresses from accessing. 2c2706d

sveitser · 2022-05-27T14:44:41Z

eqs/src/eth_polling.rs

+                        .unwrap();
+
+                    let cape_block: cap_rust_sandbox::types::CapeBlock =
+                        AbiDecode::decode(filter_data.block_bytes).unwrap();


This fails at runtime

Unpacking universal parameters... done in 9791 ms Sent funding tx to deployer xfr: 987 bytes mint: 971 bytes freeze: 979 bytes Event BlockCommittedFilter { height: 1, deposit_commitments: [], block_bytes: Bytes(b"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0 \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\xe0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01 \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\t\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\t \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\t@\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0 \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x05\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x05@#\x99\xce\xbf\xc8N\x1e\xbc\xd7\xe4\xd9L\x9f\x03_'\xdaQ\xc28\xb1\x07/lt8\x0b\xbfq\x83\x07\xdd\x1b\x069\"R\xb0\x11l2\x0f\xe8\x8f\x07\xb8\x80\xb1\x8d\xc9\xae\xf8\x14\x92|l\x15\x9cV\x8c[\xd4\xeb\x8b\r_a\xce\xa3\xdf\xdc\x8f\xc7\x0fO\xeby\x8d\xc2\x18Lvp#Ya\x91h\xfc\x14\x99\xd1y\xc5H\xae\x07t/\xf6\x98\xd4\x08\x15D\xf5\x188'\xf2H\xb1\xc74\xfbD\x19\x83p\xf0IPIg\x17\x88\xa51\x06\xb6t\x14C7\x1fx_vY\xc1\xc7\xd2QQ\x8f\xe5\xb2iS\xf0X\xc3\x0e\xe9\xea\x07\x17\x93o\x11\x0f\x8a\x0e\xcbg\x11y\xd7x5\xcaL[Ak,\x1f\x07\xe0\x84\x80?\x9e\\Nxm\x03\x1a\xa0\xb3\xfa%#yK\xba\xfaWx\x93\0\xbdq8\x87\x88\xd6\xd2\xb8]\xca\x13(\xa8\x7f)\x86N\xfa\xf1v\xdb\x98\x15\x1bCN\x8ef\x1c3u\xc3\xdc\xec\xddpn\xbf\xb3^(2\x10\x95\xa2\xad\xda \x94\xe6\xb7>O)\x14\x8f\xd9\xf4<\xb3\xbcXI!\xc3\xad`\xaf4C\x12P\x82n{\x05[\xdc_\xf9\x03\x84\xad\xf1\xeb\xbc\x1d\x19E]\rf{\xcf\xd9\xc5\xf1<xM\xd4\x12\xcco\xf2l\x15A\xe1\x95U\xcay3\xc7_t|\x0e\xc2P\xe0n\xaf*7\x0f\xc9'\xf2\x07<h]\xf0y\xaa\xb2\xa5\xb4\xaby\xb2R[S\x0eL\xaaO\x0f[\"r<Yl=\xdb\x17\xab\xeb\xb8\x19\xbaUa\x9e\n\xd12A\xd0\x8f\xd7\xe0\xd4\xaa\xb42\xady\x02*\x82\x9cU\xa6\xd6|hgh\xafb\xbb\xa1\xa5\x87\xe4L\x9a\xabD1\xb2\x074\tl\xf5\x19k\xbf\x04\xab\x14\xdd;f\x82B\xe3N\x0f\xbb\xc5\xf8\xd4\xc5D\x0f\xc3\xb2\x16\xae\x11\xaa\x8d\xe8o\x83i'\x15W\x12E\xd8_\x83\x18\x0e\x01\xaf\xdf\xe8\xc3:\xe2K/G\x9b\xc6?Y\x1f\xde\x0f\xc5x\x83\x12\x96\x84\xeb\xf3\x19\x92ff3*\xea\xf8\xb5\x9d~Y\xb9\xb6\\\xdd\xf7\x0f\x0b60^\xb3\xf7\xf6t\x0e\xf1^\x96:\x1b07X\xb7\x9c\xd1\xb0b}\x07\x1e\xa7\xfbj\xa1\n\x1ckT\xd8\xee\x95\xe0\xad\xfa6\xb2 \x89\t'l\x02\xe8a\xce\xf5\xc2_\xdb4\x8a\xa3e\xd8RKx4\xe9\xb0c\xabCL\x1b\xc3hD\xdeN\t\x0e\xc4\t\xb2-\xe0\xfa\x0c\xd9\x82\xe9\x97?\xb3-\xf1\xd2\xbb\x99\0\x99\x1c\xb0Ve\xe70\\\x88$\xf5\tDF\x13\xa4\x15)l@9\x19\x85\xb9\xc6-\x823\x83\xfd\xf4\xfb\xb2\xf2t\xfc8\x93x\x9a\xe0n\xe7\x91\x95`\x03\x18\x1b\xe8\x11\xc5a\xad9z\x1e\xd5\xd1&u\xfbZ\xa3\x03>\x98\xd6\xd2\x95\xabc\xae\x10Ft^p\x1c\xc3\xbe\xdd\t\n\xea\x96\x17\xc3\xc1\xd9ci\0=\xee\xc5\xc0\x15\x12i\xb7\x96\xa6t\x9e\x89\x1b\xd9\x13\xc6-|\xe6\xcb1\x1c\xe0\x87\x05\xaeP\xcd\xd9\x11\x19\xb8\xd1\x18\xae<'\x065iZ\x99W \xbd\x02+\x88\x07\0'\x1f\r@\xefV\xee\xa2\x1a\x1f\xea\xac\xec\xf2L\xa4\xf9\xab\x06\xf4~V\xff\xe3\xc1\xc0\xa2\x83\xcb\xb9\x06\x01]R\xf9\x05\xfe\xce\xdc~w\x95<\x03\xfdM\x0b\xfd\xe9\xcb\x91N\\\x9dGG\x13\x88)\r\xc1\xb7\x10\xa45[t$\x91\x05`\xc4\xfd\xefN\xbb\xaa\x92\xf8\\%\x9c\xe3\xea\x1a\xbaz[\xd0\\\xf4\x06\xb4\x15\x0c\xb0p\x18f\x83\xbf\x02\xc1|b~\t\xc3\xe6\xe6H\x1b\xd6{\xbd\x92\xea\xff\xa9\x12\\\xa6\x9bX9\xb6\x11A\xd54\xa2\xd5\xd3)\x16abZ\xb43\\\x8c7\xe7\x83\xb9\xf3\x14U\xe0g\x1d\xa1&\xc9K\xc3d\x1c\x8a\x92x\xe9\xb7Q\x87\xabi\xa03\x9a\xc9rgNw\xf7\xf4\xf66Hw\xcc\x1a7\x13E*\xd4&\r\xe9ejI\xdeZ\xcc\xdf\xa6gT\x1d}\x1cO\x18\xcf\x88MB\xf1!k*\x80c'@d\xb4?\x10\xfd\x07\xc3\x9a&i!l\xf7\xc3\xf2\x9b'\xbbrY\x02wz\xd04\x1f/aTB\x85Xx\xf10)'\xfd\xd6\xdc\xb9\xdb\xe9z\x13\x8d\xae\x913\xf3\"g\n\xa9\x1f\xc0\xc0h\xa5\x92\xff\xc4\x02\x81q\x9d\x8f\x1c\x9b\\?\xa5\xed[\xf4\xe1\x9e\xa9\xeaLt\n\xb8\x0f'\xcc\xa2\xb01\xa6k%H/\xb3u\xbcI\xb3,\xc2t\xaf\x9a\xbb\xea\x93\xb3Fs\x90\xb7\xed\xca}k<o\xfd\xda\x0c\xd8\x99\xbd\xc9D<~\xffJr\x17\xc8\xc1\x85\xff3\x0cI\xa2\xb3\xd9\xbb_\x1f\x14\x17|t\x95\xb6\xc5\x16\x12M\xedg\xfe>\x11\xcd\xb4=\x17\x83Z\xa9N\xa9WU\xbc%\x9f\xf6\x11#\xaa\x07\x1a\x87\xe7\xfct\x06:\x16pa\xf0y\x81W\x1e\xb5\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x05\xa0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x06\xc0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01/u`\xbd{\x90lGM\x9b\xf3\x91\xd6\x8e\xf0a\xbctJm{\xd0:\xba>|;\x17\xa2\xbe\xa6\xf3\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x02\x1d\"\x96\xce\x0f\x821\x1b\xc2YS$S.\xd2\xe8J'\xab\xce.1\xce\x7f\xcb\x7f\xda^![\xb8\xe7$\xd070\x17|1\xc0\x93O?\x10\xa3Q\x0bMvnd\xd9\xdd\xc0\x19\0\xb0\x14\x0bo_\x13Y\xa7 \xf6\xfe\x92\xb2?1Y+4?\x96\xdc\xb5\x11\xde\x80\xd6\xde\xc2\x0c\xb8\x06\xd6m\x14V\xd8\xf1\xc3\xd5\x86\x0e\xd3\xf6.\xf9e\x9d\xca_\xde\x19\xd6\xab\x12\x99`\xe5\x15\x9f_\xd6~\xe3Z\x97\x07\x03\x0b\xba-yk\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0`\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x05\x10\xd4}\xad\x13\xd8\xc1\x9aB*\xc8\xc224\x1d]\xd3\xc4\x98\xc4E\xfc\xee\xe9\x1b\x01\x9c\x984\x13w\xe4\x0f\x14R\x9fKlY\x80\x1bz\xc4D\xb0e\xc9Z\xa7\xc4\xe1\xf3\x1dp\xbd\xbc\xe5\xaa\xc3\xbb\xc7\xdcjC 6\xcd\xfc\xa9\x0e\xdd\x95\xbb%\x0f\xd1='!F\xc4\xe9N\xd1\xeb\r|[\xa3C\xcc\xfb\xeax\xdb\x0b'2V\xc1+W\xff\xc7\x88\xc2\xd9|\x18\xc5\x81\x9b\x8c\xc0\xfd\xa1Dg\xa7\xf1vH\x93\x81\xc1\xec\xd7\x0f+\xb3hg]\x0eRdL\x9b\xa6J\xd6U\xb1f\xb2\xd9\x94\xdf\xba%\xa9#\xd7\x18\xeb\x1f+\xe8ZX&\x95\x96{q\xd6\xc7Fw\xdb$\n9\x10*\xd3\xebq\x9e\r\xff\xd5)\xfd\x8d\x81^\x8b\xc29\x86\x12\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x0f\xff\xff\x15\xda']\xde\x83\xef\x1a\x98\x9ff\xae\\\x99\x87\xdd\x95r\xe2\x9f\xbc\xc8\xdchA\x16\xac\xfc\xba\xf5\xb7\xce$\xd3\xe7\xbe\x0fw\xc9\xffet\xa2\x06i\xca\x99\xab\xd4;\x8e\x0f\xc4\xc6\xfb\xdd\x0f\x0f\x1bFBV\x8cE\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\xc0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0") } thread 'async-std/runtime' panicked at 'called `Result::unwrap()` on an `Err` value: DecodingError(InvalidData)', /home/lulu/r/EspressoSystems/cape/eqs/src/eth_polling.rs:214:76 stack backtrace: 0: rust_begin_unwind at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:584:5 1: core::panicking::panic_fmt at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:143:14 2: core::result::unwrap_failed at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/result.rs:1749:5 3: eqs::eth_polling::EthPolling::check_range::{{closure}} 4: eqs::entry::run::{{closure}} 5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll 6: async_task::raw::RawTask<F,T,S>::run 7: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll 8: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll 9: async_io::driver::block_on 10: async_global_executor::reactor::block_on 11: async_global_executor::threading::thread_main_loop note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

sveitser · 2022-05-30T14:18:36Z

If we emit the abi encoded CapeBlock, decoding the data in rust fails with DecodingError(InvalidData) (see previous comment).

Doing encoding and decoding in rust with AbiEncode and AbiDecode seems to work correctly but the the output of AbiEncode does not match the bytes that we get when we do abi.encode in the contract and emit the result in an event.

We're using this approach for the record opening where it works fine.

When trying to emit the struct (instead of the abi encoded struct) compilation fails with a strange error

error: File name too long (os error 36)
    --> contracts/rust/src/bindings/mod.rs:1213:15

Could be related to the open issue:

Struct inside struct in event make cargo build fail gakonst/ethers-rs#1220

If we emit the CapeBlock struct directly this leads to a compilation error. The workaround to emit ABI encoded bytes instead fails to decode the CapeBlock in rust struct at runtime. This workaround emits the ABI encoded fields of CapeBlock individually. The function `decode_cape_block` uses an ugly workaround to deal with the problem of not being able to import `BlockCommittedFilter` because that type exists both for `CAPE` and `TestCAPE` and is therefore ambiguous.

sveitser · 2022-05-31T12:52:38Z

contracts/rust/src/cape/events.rs

+// is one definition for the CAPE and CAPETest contract.
+//
+// Is there a better workaround?
+pub fn decode_cape_block(event: CAPEEvents) -> CapeBlock {


@nyospe Wondering if you can think of a better workaround. The problem is that I would like to write a function that takes a BlockCommittedFilter parameter. However that type cannot be imported because the generated code contains duplicate definitions for it (one for CAPE and one for TestCAPE). The CAPEEvents enum has a variant that has this type as parameter so I'm using that now. As a result the function has to accept any of the CAPEEvents variants.

It would be nice if we didn't have to panic at runtime.

There's an open issue about this problem in ethers-rs.

…-data

sveitser · 2022-06-01T14:39:26Z

The CI fails because main now has a check for the contract size limit. Adding the extra data to the event significantly increases the contract size.

- Create ownable merkle tree contract owned by CAPE.

- Add missing public getters. - Remove tree height constructor arg from CAPE.

After running `hardhat deploy` check that 1. The merkle tree contract is owned by the CAPE contract. 2. The deployer can no longer add elements to the merkle tree.

Using a really high (e.g. 1M) number does have a significant impact on gas usage but also leads to a contract that is too big to deploy. With 1000 runs the CAPE contract is as 22.5 kB so there is still room to make changes to the contract.

sveitser · 2022-06-02T13:47:05Z

This should be working now.

A few rough edges:

There are a few functions in the contracts that maybe shouldn't be there. For example the getters in the IRecordsMerkleTree interface. It might make sense to duplicate those into CAPE so no external calls are needed to access them. Should probably check the impact on gas usage first though.
The TestRecordsMerkleTree contract isn't really required anymore. We are using the doNothing function only in one of benchmarks.
The code to deploy the contracts is (almost) duplicated a few times.
The relayer's submit test looks outdated and might not be useful anymore because other tests cover it better.

This reverts commit b716ae2. Re-enable failing compilation if CAPE code is > 24kB The TestCAPE contract is still too big unless we use a very low number of optimizer runs.

philippecamacho

I think the RecordsMerkleTree contract should be aware of the CAPE contract address. Otherwise the deployer can add record commitments directly into the tree.

philippecamacho · 2022-06-02T15:40:35Z

contracts/deploy/00_cape.ts

-  // but not too much in order to free the records of a rejected/lost transaction after a reasonable amount of time.
-  const nRoots = 40;
+  let recordsMerkleTreeContract = await deploy("RecordsMerkleTree", {
+    from: deployer,


Does this mean that the deployer can call any public function of RecordsMerkleTree? Is not this insecure? I mean only the deployed CAPE contract should be allowed to call RecordsMerkleTree.updateRecordsMerkleTree right?

The idea is to transfer the ownership of the contract to the CAPE contract after deployment. So only the CAPE contract can call it. I added a rough test for this in 2c2706d

When we call the RMT contract from the cape contract msg.sender would be the CAPE contract address. The onlyOwner modifier of Ownable should prevent any other caller from running the function.

…-data

philippecamacho

LGTM

sveitser · 2022-06-02T16:22:32Z

@philippecamacho

slither has some new benign re-entrancy vulnerabilities because updateRecordsMerkleTree is now an external call.

Reentrancy in CAPE.faucetSetupForTestnet(EdOnBN254.EdOnBN254Point,bytes32) (contracts/CAPE.sol#180-207):
	External calls:
	- _recordsMerkleTree.updateRecordsMerkleTree(recordCommitments) (contracts/CAPE.sol#202)
	State variables written after the call(s):
	- _addRoot(_recordsMerkleTree.getRootValue()) (contracts/CAPE.sol#203)
		- _roots[_writeHead] = newRoot (contracts/RootStore.sol#38)
	- _addRoot(_recordsMerkleTree.getRootValue()) (contracts/CAPE.sol#203)
		- _rootsMap[_roots[_writeHead]] = false (contracts/RootStore.sol#36)
		- _rootsMap[newRoot] = true (contracts/RootStore.sol#39)
	- _addRoot(_recordsMerkleTree.getRootValue()) (contracts/CAPE.sol#203)
		- _writeHead = (_writeHead + 1) % uint64(_roots.length) (contracts/RootStore.sol#41)
Reentrancy in TestCAPE.setInitialRecordCommitments(uint256[]) (contracts/mocks/TestCAPE.sol#25-29):
	External calls:
	- _recordsMerkleTree.updateRecordsMerkleTree(elements) (contracts/mocks/TestCAPE.sol#27)
	State variables written after the call(s):
	- addRoot(_recordsMerkleTree.getRootValue()) (contracts/mocks/TestCAPE.sol#28)
		- _roots[_writeHead] = newRoot (contracts/RootStore.sol#38)
	- addRoot(_recordsMerkleTree.getRootValue()) (contracts/mocks/TestCAPE.sol#28)
		- _rootsMap[_roots[_writeHead]] = false (contracts/RootStore.sol#36)
		- _rootsMap[newRoot] = true (contracts/RootStore.sol#39)
	- addRoot(_recordsMerkleTree.getRootValue()) (contracts/mocks/TestCAPE.sol#28)
		- _writeHead = (_writeHead + 1) % uint64(_roots.length) (contracts/RootStore.sol#41)
Reentrancy in CAPE.submitCapeBlock(CAPE.CapeBlock) (contracts/CAPE.sol#261-380):
	External calls:
	- _recordsMerkleTree.updateRecordsMerkleTree(commitments.items) (contracts/CAPE.sol#368)
	State variables written after the call(s):
	- _addRoot(_recordsMerkleTree.getRootValue()) (contracts/CAPE.sol#369)
		- _roots[_writeHead] = newRoot (contracts/RootStore.sol#38)
	- _addRoot(_recordsMerkleTree.getRootValue()) (contracts/CAPE.sol#369)
		- _rootsMap[_roots[_writeHead]] = false (contracts/RootStore.sol#36)
		- _rootsMap[newRoot] = true (contracts/RootStore.sol#39)
	- _addRoot(_recordsMerkleTree.getRootValue()) (contracts/CAPE.sol#369)
		- _writeHead = (_writeHead + 1) % uint64(_roots.length) (contracts/RootStore.sol#41)
	- blockHeight += 1 (contracts/CAPE.sol#373)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2

Reentrancy in CAPE.faucetSetupForTestnet(EdOnBN254.EdOnBN254Point,bytes32) (contracts/CAPE.sol#180-207):
	External calls:
	- _recordsMerkleTree.updateRecordsMerkleTree(recordCommitments) (contracts/CAPE.sol#202)
	Event emitted after the call(s):
	- FaucetInitialized(abi.encode(ro)) (contracts/CAPE.sol#205)
Reentrancy in CAPE.submitCapeBlock(CAPE.CapeBlock) (contracts/CAPE.sol#261-380):
	External calls:
	- _recordsMerkleTree.updateRecordsMerkleTree(commitments.items) (contracts/CAPE.sol#368)
	Event emitted after the call(s):
	- BlockCommitted(blockHeight,pendingDeposits,abi.encode(newBlock.minerAddr),abi.encode(newBlock.noteTypes),abi.encode(newBlock.transferNotes),abi.encode(newBlock.mintNotes),abi.encode(newBlock.freezeNotes),abi.encode(newBlock.burnNotes)) (contracts/CAPE.sol#384-393)
		- _emitBlockEvent(newBlock) (contracts/CAPE.sol#376)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-3

Emit CapeBlock as part of BlockCommitted event

ebf7584

sveitser commented May 27, 2022

View reviewed changes

sveitser added 2 commits May 30, 2022 17:09

Run cargo update

991e0cf

sveitser commented May 31, 2022

View reviewed changes

sveitser added 2 commits June 1, 2022 12:06

cargo update to get latest ethers-rs version

26df84e

Remove ambigous type workaround

8cf6def

sveitser marked this pull request as ready for review June 1, 2022 10:49

sveitser requested review from nyospe, tri-joe and philippecamacho June 1, 2022 10:49

sveitser added 2 commits June 1, 2022 12:54

Organize imports

ecaca68

Merge remote-tracking branch 'origin/main' into feat/cape-block-event…

44a848c

…-data

sveitser marked this pull request as draft June 1, 2022 14:38

sveitser added 8 commits June 1, 2022 20:42

[WIP] Separate merkle tree contract

54f6cf0

- Create ownable merkle tree contract owned by CAPE.

undo unwanted changes

15e4fa2

Add missing file

a0ca2b5

All tests pass.

f6f92a8

- Add missing public getters. - Remove tree height constructor arg from CAPE.

Update hardcoded contract addresses

c02e4df

Check access control of merkle tree contract

2c2706d

After running `hardhat deploy` check that 1. The merkle tree contract is owned by the CAPE contract. 2. The deployer can no longer add elements to the merkle tree.

Fix typescript tests

a940298

Fix typo

35edb3a

sveitser changed the title ~~Emit CapeBlock as part of BlockCommitted event~~ Split out MerkleTree contract and emit CapeBlock as part of BlockCommitted event Jun 2, 2022

sveitser added 3 commits June 2, 2022 15:24

Use 1000 solc optimizer runs instead of 20

179d4e3

Using a really high (e.g. 1M) number does have a significant impact on gas usage but also leads to a contract that is too big to deploy. With 1000 runs the CAPE contract is as 22.5 kB so there is still room to make changes to the contract.

Build executables before exporting wallet docs

9d3a60c

More consistency

1ea1f8a

Remove the geth patch that enables >24kB contracts

b716ae2

Revert "Remove the geth patch that enables >24kB contracts"

8fffead

This reverts commit b716ae2. Re-enable failing compilation if CAPE code is > 24kB The TestCAPE contract is still too big unless we use a very low number of optimizer runs.

sveitser mentioned this pull request Jun 2, 2022

Generate contracts UML diagram with sol2uml #1105

Merged

sveitser marked this pull request as ready for review June 2, 2022 15:19

philippecamacho suggested changes Jun 2, 2022

View reviewed changes

sveitser added 2 commits June 2, 2022 17:59

Merge remote-tracking branch 'origin/main' into feat/cape-block-event…

ecfc388

…-data

Update smart contract architecture diagram

353d1bd

philippecamacho self-requested a review June 2, 2022 16:19

philippecamacho approved these changes Jun 2, 2022

View reviewed changes

sveitser merged commit 60e6557 into main Jun 3, 2022

sveitser deleted the feat/cape-block-event-data branch June 3, 2022 11:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split out MerkleTree contract and emit CapeBlock as part of BlockCommitted event #1097

Split out MerkleTree contract and emit CapeBlock as part of BlockCommitted event #1097

sveitser commented May 27, 2022 •

edited

Loading

sveitser May 27, 2022

sveitser commented May 30, 2022 •

edited

Loading

sveitser May 31, 2022 •

edited

Loading

sveitser commented Jun 1, 2022

sveitser commented Jun 2, 2022 •

edited

Loading

philippecamacho left a comment

philippecamacho Jun 2, 2022

sveitser Jun 2, 2022

sveitser Jun 2, 2022

philippecamacho left a comment

sveitser commented Jun 2, 2022

Split out MerkleTree contract and emit CapeBlock as part of BlockCommitted event #1097

Split out MerkleTree contract and emit CapeBlock as part of BlockCommitted event #1097

Conversation

sveitser commented May 27, 2022 • edited Loading

TODO

sveitser May 27, 2022

Choose a reason for hiding this comment

sveitser commented May 30, 2022 • edited Loading

sveitser May 31, 2022 • edited Loading

Choose a reason for hiding this comment

sveitser commented Jun 1, 2022

sveitser commented Jun 2, 2022 • edited Loading

philippecamacho left a comment

Choose a reason for hiding this comment

philippecamacho Jun 2, 2022

Choose a reason for hiding this comment

sveitser Jun 2, 2022

Choose a reason for hiding this comment

sveitser Jun 2, 2022

Choose a reason for hiding this comment

philippecamacho left a comment

Choose a reason for hiding this comment

sveitser commented Jun 2, 2022

sveitser commented May 27, 2022 •

edited

Loading

sveitser commented May 30, 2022 •

edited

Loading

sveitser May 31, 2022 •

edited

Loading

sveitser commented Jun 2, 2022 •

edited

Loading